Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sympy-extension: render mutable matrix #2430

Closed
wants to merge 2 commits into from

Conversation

jenshnielsen
Copy link
Contributor

In sympy 0.7.2dev the default matrix class is now MutableMatrix not Matrix.
This pull request allows sympyextension to also render this class.

Matrix is an alias of MutableMatrix. Possible candidate for backport to 0.13.1
to match the next sympy release.
http://docs.sympy.org/dev/modules/matrices.html#sympy.matrices.matrices.Matrix

…atrix. This

change allows sympyextension to also render this class
@takluyver
Copy link
Member

Would it make more sense for the sympyprinting extension to live as part of SymPy, so it could easily be kept in sync with changes like this?

@takluyver
Copy link
Member

To explain more: any importable module with a load_ipython_extension function can be used as an extension, so it could live as e.g. sympy.printing. Extension docs are here: http://ipython.org/ipython-doc/stable/config/extensions/index.html#writing-extensions

@jenshnielsen
Copy link
Contributor Author

I think that is a good idea. How should we go about doing this. Will having the extension living both in sympy and ipython for backwards compatibility with sympy 0.7.1 result in issues? i.e. is there any good way to ensure that the most resent version is loaded?

@takluyver
Copy link
Member

I guess we'd do something like this:

  • Copy this module into Sympy as sympy.printing (or whatever it makes sense to call it). Make whatever changes to work with current Sympy.
  • Within the version remaining in IPython, load_ipython_extension() should try to import sympy.printing - if that succeeds, it will print a "deprecated" message pointing to the new name and call that module's load_ipython_extension(). If it fails, it falls back to the code in this module (i.e. assuming that SymPy is an older version).
  • Once we think everyone's using the new version, we can delete the copy within IPython.

Also, while we're working on it, it would be good to remove the global _loaded flag - we're trying to move away from the idea that there's only one IPython shell in a given process. It could set an attribute on the shell instead, like ip._sympy_extension_loaded.

@jenshnielsen
Copy link
Contributor Author

Just tried out removing the global loaded flag. This seems to work. If this is the way we want to go I will prepare a pull request against sympy for this version and see what they think.

@takluyver
Copy link
Member

I think putting it in sympy makes sense, although maybe others would disagree.

About the loaded flag - on reflection, is there any harm in simply running the loading code again if the user re-runs %load_ext? Alternatively, maybe the 'load only once' logic should be in the extension manager. I'll ask on the dev list for other opinions.

@jenshnielsen
Copy link
Contributor Author

For this extension there seems to be no problem reloading the extension, but it may be different for others. All the
extensions that ship with ipython seems to use the global _loaded flag.

@takluyver
Copy link
Member

They do for now, but I want to change that. I don't know if you're
subscribed to the dev list - I've just posted about it here:
http://mail.scipy.org/pipermail/ipython-dev/2012-September/010354.html

Thanks,
Thomas

@ellisonbg
Copy link
Member

I am +1 on moving the sympy printing extension to sympy. The global flag is only need to prevent adverse affects from reloading the extension. If there isn't a problem we reloading, the flag is not needed.

@takluyver
Copy link
Member

See #2462 for getting rid of the global flag (but I haven't touched sympyprinting in that).

@flacjacket
Copy link
Contributor

I'm not sure if you get a GitHub notification from cross referencing a pull request, but as a note for those interested, I've opened sympy/sympy#1556 to move the sympyprinting extension to SymPy.

@asmeurer
Copy link
Contributor

asmeurer commented Oct 3, 2012

This was also discussed at #1523.

@jenshnielsen
Copy link
Contributor Author

Just a small update. Sympy now has a printing extension with this problem solved in sympy.interactive.ipythonprinting
thanks to the work of @flacjacket so we don't need to do any thing here for 0.13.1 but we should provide a fallback to the sympy extension in ipython for 0.14

@asmeurer
Copy link
Contributor

Do you guys want to start a deprecation cycle for the sympy printing extension in 0.13.1? We should have the SymPy 0.7.2 final out this weekend, unless a serious blocker comes up. When would the release date for 0.13.1 be?

@flacjacket
Copy link
Contributor

See #2480

@jenshnielsen
Copy link
Contributor Author

Since #2480 is now doing this we can close this one

goodok pushed a commit to goodok/sympy that referenced this pull request Apr 6, 2014
Allows sympyprinting to render the new matrix default matrix class,
MutableMatrix.

Fix found by @jenshnielsen [1].

[1] ipython/ipython#2430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants